-
Notifications
You must be signed in to change notification settings - Fork 21
CSharp Crypto Queries #32
Conversation
Cs/crypto tests
| where | ||
| key_size = aglms.getKeySize() and | ||
| key_size < aglms.minKeySize() | ||
| select aglms, "Key size " + key_size.toString() + " is to small (min: " + aglms.minKeySize() + ")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to --> too
| key_size = aglms.getKeySize() and | ||
| key_size > aglms.maxKeySize() | ||
| select aglms, | ||
| "Key size " + key_size.toString() + " is to large for algorithm (max: " + aglms.maxKeySize() + ")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to --> too
| not any(ReturnedByMockObject mock).getAnArgument() = sink.asExpr() | ||
| } | ||
|
|
||
| override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not remove this line and line 25? If this was located in a .qll file, I would understand that it offers clients the ability to add things, but in a .ql file I would just leave it out. If it is ever needed, we can add it then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, scratch that, I guess this does something, indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still, lines 25 and 27 look a bit strange in this setting, I would remove them and directly refer to DataFlow::ExprNode and DataFlow::BarrierGuard
| */ | ||
|
|
||
| class Hardcoded extends Source { | ||
| Hardcoded() { this.getExpr().(IntLiteral).getValue().toInt() < 100000 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, IntegerLiteral as opposed to IntLiteral might cover more cases?
| abstract DataFlow::ExprNode key(); | ||
| } | ||
|
|
||
| class AsymmetricAlgorithm extends DataFlow::ExprNode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this really inherit from DataFlow::ExprNode? This might be a case for an algebraic datatype.
|
Did you make Sebastian's suggested changes yet? |
|
This PR is being closed and multiple smaller PRs will be coming in the next few days |
No description provided.